Inbox filter bug that puts unnecessary load on the auth backend #3449
Conversation
The part about the inbox owner existence is actually a condition of inbox taking the message or not, so that could be moved into a single filtering function. Also simplify specs by reusing types.
This needs to verify the type of the request at the moment where we need to decide if the inbox owner exists. We can't return false because that would drop the required markers from resetting the count. We can't return `can_access_room` because then the message telling the user that he has been removed from a room would arrive to the user at a time when he's not a member of the room anymore. So we simply must return true from here.
Codecov Report
@@ Coverage Diff @@
## master #3449 +/- ##
==========================================
- Coverage 80.85% 80.82% -0.04%
==========================================
Files 415 415
Lines 32312 32317 +5
==========================================
- Hits 26125 26119 -6
- Misses 6187 6198 +11
Continue to review full report at Codecov.
|
|
small_tests_24 / small_tests / ed78685 small_tests_23 / small_tests / ed78685 dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / ed78685 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / ed78685 dynamic_domains_mysql_redis_24 / mysql_redis / ed78685 dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / ed78685 ldap_mnesia_24 / ldap_mnesia / ed78685 ldap_mnesia_23 / ldap_mnesia / ed78685 sm_SUITE:parallel_manual_ack_freq_1:resume_session_state_send_message{error,
{{assertion_failed,assert_many,true,
[#Fun<sm_SUITE.22.40422599>,#Fun<sm_SUITE.22.40422599>,
#Fun<sm_SUITE.22.40422599>],
[{xmlel,<<"presence">>,
[{<<"from">>,
<<"alicE_resume_session_state_send_message_1196@localhost/escalus-default-resource">>},
{<<"to">>,
<<"alice_resume_session_state_send_message_1196@localhost/escalus-default-resource">>},
{<<"xml:lang">>,<<"en">>}],
[]},
{xmlel,<<"message">>,
[{<<"from">>,
<<"bOb_resume_session_state_send_message_1189@localhost/escalus-default-resource">>},
{<<"to">>,
<<"alicE_resume_session_state_send_message_1196@localhost">>},
{<<"xml:lang">>,<<"en">>},
{<<"type">>,<<"chat">>}],
[{xmlel,<<"body">>,[],[{xmlcdata,<<"msg-1">>}]},
{xmlel,<<"delay">>,
[{<<"xmlns">>,<<"urn:xmpp:delay">>},
{<<"stamp">>,<<"2021-12-13T10:46:33.286028Z">>},
{<<"from">>,<<"localhost">>}],
[{xmlcdata,<<"SM Storage">>}]}]},
{xmlel,<<"message">>,
[{<<"from">>,
<<"bOb_resume_session_state_send_message_1189@localhost/escalus-default-resource">>},
{<<"to">>,
<<"alicE_resume_session_state_send_message_1196@localhost">>},
{<<"xml:lang">>,<<"en">>},
{<<"type">>,<<"chat">>}],
[{xmlel,<<"body">>,[],[{xmlcdata,<<"msg-2">>}]},
{xmlel,<<"delay">>,
[{<<"xmlns">>,<<"urn:xmpp:delay">>},
{<<"stamp...internal_mnesia_24 / internal_mnesia / ed78685 pgsql_mnesia_23 / pgsql_mnesia / ed78685 elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / ed78685 pgsql_mnesia_24 / pgsql_mnesia / ed78685 mssql_mnesia_24 / odbc_mssql_mnesia / ed78685 mysql_redis_24 / mysql_redis / ed78685 riak_mnesia_24 / riak_mnesia / ed78685 |
| @@ -1031,6 +966,20 @@ groupchat_markers_all_reset_room_created(Config) -> | |||
| inbox_helper:foreach_check_inbox([Bob, Kate, Alice], 0, AliceRoomJid, Msg) | |||
| end). | |||
|
|
|||
| inbox_does_not_trigger_does_user_exist(Config) -> | |||
There was a problem hiding this comment.
what with the case, when some jid is in the room, but this jid does not exist?
|
small_tests_24 / small_tests / 61d90d0 small_tests_23 / small_tests / 61d90d0 dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 61d90d0 dynamic_domains_mysql_redis_24 / mysql_redis / 61d90d0 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 61d90d0 dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 61d90d0 ldap_mnesia_23 / ldap_mnesia / 61d90d0 ldap_mnesia_24 / ldap_mnesia / 61d90d0 pgsql_mnesia_23 / pgsql_mnesia / 61d90d0 internal_mnesia_24 / internal_mnesia / 61d90d0 mssql_mnesia_24 / odbc_mssql_mnesia / 61d90d0 pgsql_mnesia_24 / pgsql_mnesia / 61d90d0 elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 61d90d0 mysql_redis_24 / mysql_redis / 61d90d0 riak_mnesia_24 / riak_mnesia / 61d90d0 |
This fixes a bug when inbox puts a muclight room into
mongoose_hooks:does_user_exist, which makes no sense and puts unnecessary load on the auth backend. I believe this was already an issue since the beginning of inbox, but only now Beekeeper reported it.